fix: persist plan results before publishing success - #6848
Open
chenrui333 wants to merge 2 commits into
Open
Conversation
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall line coverage in commit 72d59f4 in the Show a line coverage summary of the most covered files.
Updated |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The ordering change is well-scoped and backed by deterministic tests; only a minor logging-nit was identified.
Pull request overview
This PR tightens Atlantis’s plan/autoplan lifecycle by ensuring PullStatus is durably persisted before publishing any successful plan output (success comment, per-project plan checks, and aggregate plan/apply checks), and by failing loudly/explicitly when persistence fails to avoid misleading “success” signals.
Changes:
- Defers successful per-project plan status updates until after the DB write boundary (while still emitting failed statuses immediately).
- Reorders plan/autoplan publication so the success comment and aggregate statuses are only published after PullStatus persistence succeeds; on persistence failure, publishes an actionable error and marks plan/apply aggregates as failed.
- Adds deterministic regression tests covering ordering for manual plan/autoplan and persistence-write failures via production wrappers.
File summaries
| File | Description |
|---|---|
| server/events/project_command_runner.go | Adds deferred plan-status publishing and defers non-API plan success project checks until after persistence. |
| server/events/plan_command_runner.go | Reorders plan/autoplan publication to persist first; adds persistence-failure handler and deferred plan-status publishing hook. |
| server/events/instrumented_project_command_runner.go | Passes through the new deferred plan-status publisher interface for instrumented runners. |
| server/events/command_runner_test.go | Allows tests to wrap the plan runner independently (to observe/verify persistence boundary behavior). |
| server/events/plan_command_runner_test.go | Adds deterministic tests asserting persistence happens before success publication and that write failures fail plan/apply aggregates with actionable messaging. |
| server/events/project_command_runner_test.go | Updates expectations so plan success project checks are deferred in non-API flows (and apply success remains deferred). |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 7, 2026
Addresses #6641 Assisted-by: OpenAI GPT-6 <noreply@openai.com> Signed-off-by: Rui Chen <rui@chenrui.dev>
Assisted-by: OpenAI GPT-6 <noreply@openai.com> Signed-off-by: Rui Chen <rui@chenrui.dev>
chenrui333
force-pushed
the
fix/persist-plan-before-success
branch
from
September 7, 2026 23:54
7c1e948 to
72d59f4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Persist plan/autoplan PullStatus before publishing the success comment or successful per-project and aggregate checks. If persistence fails, publish an actionable error and fail the plan/apply aggregates instead of leaving a misleading success.
Addresses #6641.
Notes
Extracts the persistence-ordering invariant from #6657, without generations, publication claims, or plan-store changes. Existing empty/targeted-plan and cleanup behavior is preserved. #6781 is already on the base and is not duplicated.
Deterministic tests cover manual/autoplan ordering and write failures through the production output/metrics wrappers. Existing targeted-empty-plan status-preservation tests remain in place. Local lint reports three unchanged findings (two modernize suggestions and one deprecated GitLab test field); hosted checks are still required.